Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor doc suggestion #153

Merged
merged 4 commits into from
Jan 15, 2025
Merged

Conversation

Rafnuss
Copy link
Contributor

@Rafnuss Rafnuss commented Jan 5, 2025

I noticed that CDS is now calling the "Personal Access token" a "API token". It's been a bit confusing, but probably best stick to their name convension, right?

Here are some minor edits (feel free to reject, not offended).

Also, in terms of structure, I could see the heading "Use: ECMWF Data Store services" becoming "Setup" including the content for retrieving the token within this section and start a new level 2 section for Data Requests

"File and system based keychains" sounds more like for the advanced_vignette as well as the issue with date which is now present in both.

@khufkens
Copy link
Member

Thanks for the additions. Check why the tests fail as of commit fcfe180 - something is funky from then on.

I would like to keep the subheader Use as it is what I implemented everywhere. Consistency matters I think. What I can see is expand the the intro section to include more details of the online setup, in contrast to the setup section of the R software itself below. If this makes sense.

@Rafnuss
Copy link
Contributor Author

Rafnuss commented Jan 15, 2025

 ── Error ('test_ds.R:244:3'): batch request tests ──────────────────────────────
 Error: Error: permission deniedpermission denied401authentication requiredhttps://cds.climate.copernicus.eu/api/retrieve/v1/processes/reanalysis-era5-pressure-levels/executecb9139ca-a082-4cd1-9cac-51896838dd22

I'm actually not sure what is going wrong here. I don't really understand how the authentification is working in the test. In any case, I also don't see the link with my modifications... Sorry...

@khufkens
Copy link
Member

My local version runs all checks fine, not sure why the remote version doesn't work. I'll merge and check out changes locally as well.

@khufkens khufkens merged commit 390e9be into bluegreen-labs:master Jan 15, 2025
1 of 7 checks passed
@khufkens
Copy link
Member

Passes when pulled. I've been developing for a while alone now so I think this always happens as I use encrypted credentials (environmental variables) on my part for authentication. Since this comes from "outside" as a pull request the tests don't have access to those (otherwise a malicious pull request could exfiltrate these).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants